Use git submodules for (securely) using third party Github Actions. #13514

ashb · 2021-01-06T12:36:03Z

This approach gives us the ability to keep track of the actions we use, doesn't fall foul of the "no third party actions" org-wide setting, and still gives us "audit-by-default" -- any change to the commit of a submodule would need a PR and GitHub handles this quite nicely on the PR review.

Note: I an still generally against submodules as they are difficult to use, but since this is only for CI and everyone* else can checkout the repo and not have to care that the submodules exist I think it the right tool for this use case.

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

ashb · 2021-01-06T12:43:48Z

See https://github.com/astronomer/airflow/pull/1171/files for what the review process looks like for when a submodule is updated in a PR. (I downgraded that submodule to v1_2 in that draft PR)

.gitmodules

kaxil

This looks good, better than maintaining a separate repo for each action

ashb · 2021-01-07T10:44:29Z

Could someone please double check my changes are right, in particular the build-images one -- I think it should be running the actions from the main branch, not the PR branch.

potiuk · 2021-01-07T11:49:01Z

Could someone please double check my changes are right, in particular the build-images one -- I think it should be running the actions from the main branch, not the PR branch.

You can check it yourself, just push to your fork as main branch (you have to have github actions enabled). I will do it now as well, but this is the easiest one for you to check it.

ashb · 2021-01-07T11:49:58Z

Ah cool yes, I'll check that.

potiuk · 2021-01-07T11:52:37Z

Running here: https://github.com/potiuk/airflow/actions/runs/468787325

potiuk · 2021-01-07T11:55:04Z

I do not see the code from submodules in the review @ashb . Do you know why it is so ?

ashb · 2021-01-07T11:55:43Z

Hmm, what do I have to do to turn actions on for my fork? I don't see an option to enable/disable actions in my settings. Must be missing it.

Oh found it - the workflow was manually disabled

potiuk · 2021-01-07T11:57:07Z

This is what I see rather than list of files:

Not a deal breaker. But would be nice to have it.

ashb · 2021-01-07T11:58:28Z

Yeah -for adding a new submodule it shows that. Updating a submodule shows the full/better view ashb#3

potiuk · 2021-01-07T11:59:24Z

Yeah -for adding a new submodule it shows that. Updating a submodule shows the full/better view ashb#3

Are you sure it works from the forks? I suspect it is only for PRs in the same repo.

potiuk · 2021-01-07T12:00:56Z

Are you sure it works from the forks? I suspect it is only for PRs in the same repo.

I will push it as a branch to 'apache/airflow' and see if it looks better. We can have a rule that only committers make changes to actions and only do it via internal PR if we find that this is the case

ashb · 2021-01-07T12:01:15Z

@potiuk astronomer#1174

Cross repo PR that updates a sub-module ^^

potiuk · 2021-01-07T12:04:33Z

Cross repo PR that updates a sub-module ^^

Yep. just tested it :). All Good !

ashb · 2021-01-07T12:06:10Z

Unrelated to this PR, but I thought that manually setting "full tests needed" label on my PR ashb#3 and then re-running it should have made it run everything, but it still only ran minimal tests

Fixed by #13538

.github/workflows/build-images-workflow-run.yml

potiuk

Looks good. Build passed in my fork:
https://github.com/potiuk/airflow/actions/runs/468787325

And CI builds are about to pass as well:
https://github.com/potiuk/airflow/actions/runs/468787281

github-actions · 2021-01-07T12:56:29Z

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

Rather than having to mirror all the repos we can instead use git submodules to pull in the third party actions we want to use - with recent(ish) changes in review for submodules on GitHub we still get the same "review/audit" visibility for changes, but this way we don't have to either "pollute" our repo with the actions code, nor do we have to maintain a fork of the third party action.

The most recent submodule change for actions apache#13514 was done in parallel to Optimising worklfows in apache#13562 and the job added in the apache#13562 still uses non-submodule version of check action. Also few checkout steps missed: 'submodules: recursive' input This PR fixes that and all 3rd-party actions now are used from submodule.

The most recent submodule change for actions #13514 was done in parallel to Optimising worklfows in #13562 and the job added in the #13562 still uses non-submodule version of check action. Also few checkout steps missed: 'submodules: recursive' input This PR fixes that and all 3rd-party actions now are used from submodule.

Rather than having to mirror all the repos we can instead use git submodules to pull in the third party actions we want to use - with recent(ish) changes in review for submodules on GitHub we still get the same "review/audit" visibility for changes, but this way we don't have to either "pollute" our repo with the actions code, nor do we have to maintain a fork of the third party action. (cherry picked from commit f115983)

The most recent submodule change for actions #13514 was done in parallel to Optimising worklfows in #13562 and the job added in the #13562 still uses non-submodule version of check action. Also few checkout steps missed: 'submodules: recursive' input This PR fixes that and all 3rd-party actions now are used from submodule. (cherry picked from commit eb40eea)

Rather than having to mirror all the repos we can instead use git submodules to pull in the third party actions we want to use - with recent(ish) changes in review for submodules on GitHub we still get the same "review/audit" visibility for changes, but this way we don't have to either "pollute" our repo with the actions code, nor do we have to maintain a fork of the third party action. (cherry picked from commit f115983)

Rather than having to mirror all the repos we can instead use git submodules to pull in the third party actions we want to use - with recent(ish) changes in review for submodules on GitHub we still get the same "review/audit" visibility for changes, but this way we don't have to either "pollute" our repo with the actions code, nor do we have to maintain a fork of the third party action. (cherry picked from commit f115983) (cherry picked from commit eddd632)

The most recent submodule change for actions apache#13514 was done in parallel to Optimising worklfows in apache#13562 and the job added in the apache#13562 still uses non-submodule version of check action. Also few checkout steps missed: 'submodules: recursive' input This PR fixes that and all 3rd-party actions now are used from submodule. (cherry picked from commit eb40eea) (cherry picked from commit c8895bd)

RA80533 · 2021-06-05T11:29:46Z

@ashb How did this idea come about? This is the only project I've come across that partitions their third-party Actions in such a manner. Was there any prior attempts at this anywhere else?

potiuk · 2021-06-05T11:43:04Z

@ashb How did this idea come about? This is the only project I've come across that partitions their third-party Actions in such a manner. Was there any prior attempts at this anywhere else?

This was triggered by a potential supply-chain attack that can be issued when you are integrating GitHub Actions via tag or branch. Submodule solution automatically implements all the recommendations described by Github Actions here https://docs.github.com/en/actions/learn-github-actions/security-hardening-for-github-actions#using-third-party-actions , including the fact that it is very easy to review any PRs where actions are updated (such PR will allow to review the changes in the action as submodules are nicely integrated in GitHub PR review process).

You can read summary of the GitHub Action status here - and all context and explanations for recommendations we came up with and proposed to other ASF projects:

https://cwiki.apache.org/confluence/display/BUILDS/GitHub+Actions+status

ashb · 2021-06-05T11:45:16Z

@RA80533 this brought on a setting by Apache Software Foundation Infra and security teams changed on our GitHub org - they only allow Actions from official GitHub, or ASF repos.

This is to somewhat defend against "supply chain" attacks where a project uses some random action which is then updated to exfil creds etc.

potiuk · 2021-06-05T11:45:54Z

About prior art - not that I am aware of it but also Apache beam started using it after the discussion https://github.com/apache/beam/blob/master/.gitmodules

@potiuk

My change is the fix and improvement for github action which labels approved PRs (introduced in this [PR](#6239)). It is inspired by solution introduced and tested in [Apache Airflow](https://github.com/apache/airflow) (thanks @potiuk @ashb 🚀 ) Corresponding Apache Airflow workflows on which I based this PR: - https://github.com/apache/airflow/blob/main/.github/workflows/label_when_reviewed.yml - https://github.com/apache/airflow/blob/main/.github/workflows/label_when_reviewed_workflow_run.yml Problems which were solved in this PR: - **Permissions**. @morningman opened a related bug: [[Help] Error: Resource not accessible by integration](TobKed/label-when-approved-action#7). It is related to limited permissions of workflows being triggered by `pull_request_review` (`GITHUB_TOKEN` has read-only permissions). More information about it you can find in the article: [Keeping your GitHub Actions and workflows secure: Preventing pwn requests](https://securitylab.github.com/research/github-actions-preventing-pwn-requests/). TL;DR: On pull request review event (`on: pull_request_review` ) "dummy" workflow `Label when reviewed` triggers another workflow `Label when approved workflow run` which has sufficient permissions (`on: workflow_run: workflows: ["Label when reviewed"]`). - **Safe use of 3rd-party Github Actions by using submodules pattern.** It is decribed in: https://cwiki.apache.org/confluence/display/BUILDS/GitHub+Actions+status > NEVER use 3rd-party actions directly in your workflows - use the "submodule" pattern. This pattern is successfully used by projects like: - [Apache Airflow](https://github.com/apache/airflow) ([PR](apache/airflow#13514)) - [Apache Beam](https://github.com/apache/beam) ([PR](apache/beam#13736)) - [Apache Superset](https://github.com/apache/superset) ([PR](apache/superset#12709))

boring-cyborg bot added the area:dev-tools label Jan 6, 2021

ashb mentioned this pull request Jan 6, 2021

Switch external actions to subrepo'ed/submodules .github/actions/ #13513

Closed

potiuk reviewed Jan 6, 2021

View reviewed changes

.gitmodules Show resolved Hide resolved

potiuk mentioned this pull request Jan 6, 2021

[BEAM-11569] Enabling wheels to be built using Apache-based workflow apache/beam#13674

Merged

4 tasks

kaxil reviewed Jan 7, 2021

View reviewed changes

potiuk mentioned this pull request Jan 7, 2021

[BEAM-11569] Fixing cancel workflows apache/beam#13679

Merged

4 tasks

ashb force-pushed the actions-via-submodules branch from 678ba95 to 087d0a0 Compare January 7, 2021 10:39

ashb changed the title ~~WIP: Test out github actions via gitmodules~~ Use git submodules for (securely) using third party Github Actions. Jan 7, 2021

ashb marked this pull request as ready for review January 7, 2021 10:43

ashb requested review from potiuk and kaxil January 7, 2021 10:43

potiuk reviewed Jan 7, 2021

View reviewed changes

.github/workflows/build-images-workflow-run.yml Show resolved Hide resolved

kaxil approved these changes Jan 7, 2021

View reviewed changes

potiuk approved these changes Jan 7, 2021

View reviewed changes

github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Jan 7, 2021

ashb force-pushed the actions-via-submodules branch from 087d0a0 to b7423f3 Compare January 10, 2021 22:04

TobKed approved these changes Jan 11, 2021

View reviewed changes

potiuk merged commit f115983 into apache:master Jan 11, 2021

mik-laj mentioned this pull request Jan 11, 2021

Use git submodules for (securely) using third party Github Actions apache/airflow-site#372

Merged

potiuk mentioned this pull request Jan 12, 2021

Add submodules in added workflow job #13631

Merged

TobKed mentioned this pull request Jan 23, 2021

ci: use git submodules for (securely) using third party Github Actions apache/superset#12709

Merged

6 tasks

ashb deleted the actions-via-submodules branch June 5, 2021 11:45

TobKed mentioned this pull request Jul 21, 2021

[Community] Fix PR labeling github action workflow apache/doris#6279

Merged

13 tasks

hongaar mentioned this pull request Apr 11, 2022

chore: use exivity fork exivity/charts#1

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use git submodules for (securely) using third party Github Actions. #13514

Use git submodules for (securely) using third party Github Actions. #13514

ashb commented Jan 6, 2021 •

edited

Loading

ashb commented Jan 6, 2021

kaxil left a comment

ashb commented Jan 7, 2021

potiuk commented Jan 7, 2021

ashb commented Jan 7, 2021

potiuk commented Jan 7, 2021

potiuk commented Jan 7, 2021

ashb commented Jan 7, 2021 •

edited

Loading

potiuk commented Jan 7, 2021 •

edited

Loading

ashb commented Jan 7, 2021

potiuk commented Jan 7, 2021

potiuk commented Jan 7, 2021

ashb commented Jan 7, 2021 •

edited

Loading

potiuk commented Jan 7, 2021

ashb commented Jan 7, 2021 •

edited

Loading

potiuk left a comment •

edited

Loading

github-actions bot commented Jan 7, 2021

RA80533 commented Jun 5, 2021 •

edited

Loading

potiuk commented Jun 5, 2021 •

edited

Loading

ashb commented Jun 5, 2021

potiuk commented Jun 5, 2021

Use git submodules for (securely) using third party Github Actions. #13514

Use git submodules for (securely) using third party Github Actions. #13514

Conversation

ashb commented Jan 6, 2021 • edited Loading

ashb commented Jan 6, 2021

kaxil left a comment

Choose a reason for hiding this comment

ashb commented Jan 7, 2021

potiuk commented Jan 7, 2021

ashb commented Jan 7, 2021

potiuk commented Jan 7, 2021

potiuk commented Jan 7, 2021

ashb commented Jan 7, 2021 • edited Loading

potiuk commented Jan 7, 2021 • edited Loading

ashb commented Jan 7, 2021

potiuk commented Jan 7, 2021

potiuk commented Jan 7, 2021

ashb commented Jan 7, 2021 • edited Loading

potiuk commented Jan 7, 2021

ashb commented Jan 7, 2021 • edited Loading

potiuk left a comment • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Jan 7, 2021

RA80533 commented Jun 5, 2021 • edited Loading

potiuk commented Jun 5, 2021 • edited Loading

ashb commented Jun 5, 2021

potiuk commented Jun 5, 2021

ashb commented Jan 6, 2021 •

edited

Loading

ashb commented Jan 7, 2021 •

edited

Loading

potiuk commented Jan 7, 2021 •

edited

Loading

ashb commented Jan 7, 2021 •

edited

Loading

ashb commented Jan 7, 2021 •

edited

Loading

potiuk left a comment •

edited

Loading

RA80533 commented Jun 5, 2021 •

edited

Loading

potiuk commented Jun 5, 2021 •

edited

Loading